-
Notifications
You must be signed in to change notification settings - Fork 140
Make PollingInterval configurable in HTTPScaledObject #1252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…t.Spec Signed-off-by: Sebastian Penhouet <[email protected]>
wozniakjan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thank you!
ptal @zroubalik, I know you are not a big fan of duplicating fields between HTTPScaledObject and ScaledObject but imho this one also makes sense because the operator sets this to a hardcoded 15s and patches over any custom changes to the ScaledObject made by the user
zroubalik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternate proposal:
What if we just remove the hardcoded 15s and let it on ScaledObject?
This project is still beta so backwards compatibility is something we can ingore for certain cases and I think this one is a good one.
|
The add-on is connected to KEDA as external-push scaler, so the polling interval isn't so relevant indeed. I don't have in my mind all the add-on details atm, but I think that it's more or less how I have described. Which is the goal of changing the |
|
@JorTurFer Maybe I'm misunderstanding something, but our goal is to reduce the time of the polling interval to ensure fast scaling. Right now we solved it by not using the HTTPScaledObject and instead directly using a ScaledObject, which allows to set the polling interval. Adding this config enables us to remove that workaround. |
|
My point is that I'm not sure if you're actually getting an advantage in the scaling or it's just a useless parameter. I mean, if you set a crazily high value (like 300 or 3000), do you need to wait up to those seconds to scale from 0 or your workload is scaled instantly? If it's scaled instantly, the |
Yes, omg what I was thinking before 🤦♂️ 😄 Too much context-switching probably. @Spenhouet pollingInterval is not used at all by this addon, the activation happens almost immediatelly when the scaler tells KEDA to do 0->1. So this particular config doesn't affect anything. |
yeah, that is a good point, not sure how I managed to forget this...
and given this, I think it's safe to close this PR without a merge, apologies for the confusion I caused earlier |
Provide the PollingInterval of the ScaledObject as parameter in the HTTPScaledObject.
Checklist
README.mddocs/directoryFixes #1251